[Clang] Explain why is_standard_layout evaluated to false #143722

snarang181 · 2025-06-11T15:13:38Z

As part of the effort in #141911

llvmbot · 2025-06-11T15:18:41Z

@llvm/pr-subscribers-clang

Author: Samarth Narang (snarang181)

Changes

As part of the effort in #141911

Full diff: https://github.com/llvm/llvm-project/pull/143722.diff

4 Files Affected:

(modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+7-1)
(modified) clang/lib/Sema/SemaTypeTraits.cpp (+100)
(modified) clang/test/SemaCXX/type-traits-unsatisfied-diags-std.cpp (+54)
(modified) clang/test/SemaCXX/type-traits-unsatisfied-diags.cpp (+46)

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 0f77083dac9df..52221462de185 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -1767,7 +1767,8 @@ def note_unsatisfied_trait
     : Note<"%0 is not %enum_select<TraitName>{"
            "%TriviallyRelocatable{trivially relocatable}|"
            "%Replaceable{replaceable}|"
-           "%TriviallyCopyable{trivially copyable}"
+           "%TriviallyCopyable{trivially copyable}|"
+                                  "%StandardLayout{standard-layout}"
            "}1">;
 
 def note_unsatisfied_trait_reason
@@ -1787,6 +1788,11 @@ def note_unsatisfied_trait_reason
            "%NonReplaceableField{has a non-replaceable member %1 of type %2}|"
            "%NTCBase{has a non-trivially-copyable base %1}|"
            "%NTCField{has a non-trivially-copyable member %1 of type %2}|"
+           "%NonStdLayoutBase{has a non-standard-layout base %1}|"
+           "%MixedAccess{has mixed access specifiers}|"
+           "%MultipleDataBase{has multiple base classes with data members}|"
+           "%VirtualFunction{has virtual functions}|"
+           "%NonStdLayoutMember{has a non-standard-layout member %1 of type %2}|"
            "%DeletedDtr{has a %select{deleted|user-provided}1 destructor}|"
            "%UserProvidedCtr{has a user provided %select{copy|move}1 "
            "constructor}|"
diff --git a/clang/lib/Sema/SemaTypeTraits.cpp b/clang/lib/Sema/SemaTypeTraits.cpp
index 1738ab4466001..730fc77a34712 100644
--- a/clang/lib/Sema/SemaTypeTraits.cpp
+++ b/clang/lib/Sema/SemaTypeTraits.cpp
@@ -1947,6 +1947,7 @@ static std::optional<TypeTrait> StdNameToTypeTrait(StringRef Name) {
             TypeTrait::UTT_IsCppTriviallyRelocatable)
       .Case("is_replaceable", TypeTrait::UTT_IsReplaceable)
       .Case("is_trivially_copyable", TypeTrait::UTT_IsTriviallyCopyable)
+      .Case("is_standard_layout", TypeTrait::UTT_IsStandardLayout)
       .Default(std::nullopt);
 }
 
@@ -2276,6 +2277,102 @@ static void DiagnoseNonTriviallyCopyableReason(Sema &SemaRef,
   SemaRef.Diag(D->getLocation(), diag::note_defined_here) << D;
 }
 
+static bool hasMixedAccessSpecifier(const CXXRecordDecl *D) {
+  AccessSpecifier FirstAccess = AS_none;
+  for (const FieldDecl *Field : D->fields()) {
+    if (Field->isUnnamedBitField())
+      continue;
+    AccessSpecifier FieldAccess = Field->getAccess();
+    if (FirstAccess == AS_none) {
+      FirstAccess = FieldAccess;
+    } else if (FieldAccess != FirstAccess) {
+      return true;
+    }
+  }
+  return false;
+}
+
+static bool hasMultipleDataBaseClassesWithFields(const CXXRecordDecl *D) {
+  int NumBasesWithFields = 0;
+  for (const CXXBaseSpecifier &Base : D->bases()) {
+    const CXXRecordDecl *BaseRD = Base.getType()->getAsCXXRecordDecl();
+    if (!BaseRD || BaseRD->isInvalidDecl())
+      continue;
+
+    for (const FieldDecl *Field : BaseRD->fields()) {
+      if (!Field->isUnnamedBitField()) {
+        ++NumBasesWithFields;
+        break; // Only count the base once.
+      }
+    }
+  }
+  return NumBasesWithFields > 1;
+}
+
+static void DiagnoseNonStandardLayoutReason(Sema &SemaRef, SourceLocation Loc,
+                                            const CXXRecordDecl *D) {
+  for (const CXXBaseSpecifier &B : D->bases()) {
+    assert(B.getType()->getAsCXXRecordDecl() && "invalid base?");
+    if (B.isVirtual()) {
+      SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason)
+          << diag::TraitNotSatisfiedReason::VBase << B.getType()
+          << B.getSourceRange();
+    }
+    if (!B.getType()->isStandardLayoutType()) {
+      SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason)
+          << diag::TraitNotSatisfiedReason::NonStdLayoutBase << B.getType()
+          << B.getSourceRange();
+    }
+  }
+  if (hasMixedAccessSpecifier(D)) {
+    SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason)
+        << diag::TraitNotSatisfiedReason::MixedAccess;
+  }
+  if (hasMultipleDataBaseClassesWithFields(D)) {
+    SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason)
+        << diag::TraitNotSatisfiedReason::MultipleDataBase;
+  }
+  if (D->isPolymorphic()) {
+    SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason)
+        << diag::TraitNotSatisfiedReason::VirtualFunction;
+  }
+  for (const FieldDecl *Field : D->fields()) {
+    if (!Field->getType()->isStandardLayoutType()) {
+      SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason)
+          << diag::TraitNotSatisfiedReason::NonStdLayoutMember << Field
+          << Field->getType() << Field->getSourceRange();
+    }
+  }
+}
+
+static void DiagnoseNonStandardLayoutReason(Sema &SemaRef, SourceLocation Loc,
+                                            QualType T) {
+  SemaRef.Diag(Loc, diag::note_unsatisfied_trait)
+      << T << diag::TraitName::StandardLayout;
+
+  // Check type-level exclusion first
+  if (T->isVariablyModifiedType()) {
+    SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason)
+        << diag::TraitNotSatisfiedReason::VLA;
+    return;
+  }
+
+  if (T->isReferenceType()) {
+    SemaRef.Diag(Loc, diag::note_unsatisfied_trait_reason)
+        << diag::TraitNotSatisfiedReason::Ref;
+    return;
+  }
+  T = T.getNonReferenceType();
+  const CXXRecordDecl *D = T->getAsCXXRecordDecl();
+  if (!D || D->isInvalidDecl())
+    return;
+
+  if (D->hasDefinition())
+    DiagnoseNonStandardLayoutReason(SemaRef, Loc, D);
+
+  SemaRef.Diag(D->getLocation(), diag::note_defined_here) << D;
+}
+
 void Sema::DiagnoseTypeTraitDetails(const Expr *E) {
   E = E->IgnoreParenImpCasts();
   if (E->containsErrors())
@@ -2296,6 +2393,9 @@ void Sema::DiagnoseTypeTraitDetails(const Expr *E) {
   case UTT_IsTriviallyCopyable:
     DiagnoseNonTriviallyCopyableReason(*this, E->getBeginLoc(), Args[0]);
     break;
+  case UTT_IsStandardLayout:
+    DiagnoseNonStandardLayoutReason(*this, E->getBeginLoc(), Args[0]);
+    break;
   default:
     break;
   }
diff --git a/clang/test/SemaCXX/type-traits-unsatisfied-diags-std.cpp b/clang/test/SemaCXX/type-traits-unsatisfied-diags-std.cpp
index 329b611110c1d..cc10cd2405b4d 100644
--- a/clang/test/SemaCXX/type-traits-unsatisfied-diags-std.cpp
+++ b/clang/test/SemaCXX/type-traits-unsatisfied-diags-std.cpp
@@ -20,6 +20,13 @@ struct is_trivially_copyable {
 
 template <typename T>
 constexpr bool is_trivially_copyable_v = __is_trivially_copyable(T);
+
+template <typename T>
+struct is_standard_layout {
+  static constexpr bool value = __is_standard_layout(T);
+};
+template <typename T>
+constexpr bool is_standard_layout_v = __is_standard_layout(T);
 #endif
 
 #ifdef STD2
@@ -44,6 +51,15 @@ using is_trivially_copyable  = __details_is_trivially_copyable<T>;
 
 template <typename T>
 constexpr bool is_trivially_copyable_v = __is_trivially_copyable(T);
+
+template <typename T>
+struct __details_is_standard_layout {
+  static constexpr bool value = __is_standard_layout(T);
+};
+template <typename T>
+using is_standard_layout = __details_is_standard_layout<T>;
+template <typename T>
+constexpr bool is_standard_layout_v = __is_standard_layout(T);
 #endif
 
 
@@ -73,6 +89,15 @@ using is_trivially_copyable  = __details_is_trivially_copyable<T>;
 
 template <typename T>
 constexpr bool is_trivially_copyable_v = is_trivially_copyable<T>::value;
+
+
+template <typename T>
+struct __details_is_standard_layout : bool_constant<__is_standard_layout(T)> {};
+template <typename T>
+using is_standard_layout = __details_is_standard_layout<T>;
+template <typename T>
+constexpr bool is_standard_layout_v = is_standard_layout<T>::value;
+
 #endif
 
 }
@@ -100,6 +125,21 @@ static_assert(std::is_trivially_copyable_v<int&>);
 // expected-note@-1 {{because it is a reference type}}
 
 
+// Direct tests
+static_assert(std::is_standard_layout<int>::value);
+static_assert(std::is_standard_layout_v<int>);
+
+static_assert(std::is_standard_layout<int&>::value);
+// expected-error-re@-1 {{static assertion failed due to requirement 'std::{{.*}}is_standard_layout<int &>::value'}} \
+// expected-note@-1 {{'int &' is not standard-layout}} \
+// expected-note@-1 {{because it is a reference type}}
+
+static_assert(std::is_standard_layout_v<int&>);
+// expected-error@-1 {{static assertion failed due to requirement 'std::is_standard_layout_v<int &>'}} \
+// expected-note@-1 {{'int &' is not standard-layout}} \
+// expected-note@-1 {{because it is a reference type}}
+
+
 namespace test_namespace {
     using namespace std;
     static_assert(is_trivially_relocatable<int&>::value);
@@ -119,9 +159,21 @@ namespace test_namespace {
     // expected-error@-1 {{static assertion failed due to requirement 'is_trivially_copyable_v<int &>'}} \
     // expected-note@-1 {{'int &' is not trivially copyable}} \
     // expected-note@-1 {{because it is a reference type}}
+
+    static_assert(is_standard_layout<int&>::value);
+    // expected-error-re@-1 {{static assertion failed due to requirement '{{.*}}is_standard_layout<int &>::value'}} \
+    // expected-note@-1 {{'int &' is not standard-layout}} \
+    // expected-note@-1 {{because it is a reference type}}
+
+    static_assert(is_standard_layout_v<int&>);
+    // expected-error@-1 {{static assertion failed due to requirement 'is_standard_layout_v<int &>'}} \
+    // expected-note@-1 {{'int &' is not standard-layout}} \
+    // expected-note@-1 {{because it is a reference type}}
+    
 }
 
 
+
 namespace concepts {
 template <typename T>
 requires std::is_trivially_relocatable<T>::value void f();  // #cand1
@@ -192,3 +244,5 @@ static_assert(std::is_replaceable_v<int&>);
 // expected-error@-1 {{static assertion failed due to requirement 'std::is_replaceable_v<int &>'}} \
 // expected-note@-1 {{'int &' is not replaceable}} \
 // expected-note@-1 {{because it is a reference type}}
+
+
diff --git a/clang/test/SemaCXX/type-traits-unsatisfied-diags.cpp b/clang/test/SemaCXX/type-traits-unsatisfied-diags.cpp
index a8c78f6304ca9..65c376db7ea02 100644
--- a/clang/test/SemaCXX/type-traits-unsatisfied-diags.cpp
+++ b/clang/test/SemaCXX/type-traits-unsatisfied-diags.cpp
@@ -488,3 +488,49 @@ static_assert(__is_trivially_copyable(S12));
 // expected-note@-1 {{'S12' is not trivially copyable}} \
 // expected-note@#tc-S12 {{'S12' defined here}}
 }
+
+namespace standard_layout_tests {
+    struct WithVirtual { // #sl-Virtual
+  virtual void foo();
+};
+static_assert(__is_standard_layout(WithVirtual));
+// expected-error@-1 {{static assertion failed due to requirement '__is_standard_layout(standard_layout_tests::WithVirtual)'}} \
+// expected-note@-1 {{'WithVirtual' is not standard-layout}} \
+// expected-note@-1 {{because it has virtual functions}} \
+// expected-note@#sl-Virtual {{'WithVirtual' defined here}}
+
+struct MixedAccess { // #sl-Mixed
+public:
+  int a;
+private:
+  int b;
+};
+static_assert(__is_standard_layout(MixedAccess));
+// expected-error@-1 {{static assertion failed due to requirement '__is_standard_layout(standard_layout_tests::MixedAccess)'}} \
+// expected-note@-1 {{'MixedAccess' is not standard-layout}} \
+// expected-note@-1 {{because it has mixed access specifiers}} \
+// expected-note@#sl-Mixed {{'MixedAccess' defined here}}
+
+struct VirtualBase { virtual ~VirtualBase(); };               // #sl-VirtualBase
+struct VB : virtual VirtualBase {};                            // #sl-VB
+static_assert(__is_standard_layout(VB));
+// expected-error@-1 {{static assertion failed due to requirement '__is_standard_layout(standard_layout_tests::VB)'}} \
+// expected-note@-1 {{'VB' is not standard-layout}} \
+// expected-note@-1 {{because it has a virtual base 'VirtualBase'}} \
+// expected-note@-1 {{because it has a non-standard-layout base 'VirtualBase'}} \
+// expected-note@-1 {{because it has virtual functions}}
+// expected-note@#sl-VB {{'VB' defined here}}
+
+union U {      // #sl-U
+public:
+    int x;
+private:
+    int y;
+};                                                       
+static_assert(__is_standard_layout(U));
+// expected-error@-1 {{static assertion failed due to requirement '__is_standard_layout(standard_layout_tests::U)'}} \
+// expected-note@-1 {{'U' is not standard-layout}} \
+// expected-note@-1 {{because it has mixed access specifiers}}
+// expected-note@#sl-U {{'U' defined here}}
+
+}

snarang181 · 2025-06-11T15:18:44Z

@cor3ntin, requesting your review here.

Copilot

Pull Request Overview

Adds support for explaining why __is_standard_layout (and its <type_traits> equivalents) evaluates to false.

Introduce new test cases for unsatisfied standard-layout trait in type-traits-unsatisfied-diags.cpp and its <type_traits> counterpart.
Extend SemaTypeTraits implementation to diagnose all standard-layout violations.
Update diagnostics definitions to include StandardLayout in DiagnosticSemaKinds.td.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File	Description
clang/test/SemaCXX/type-traits-unsatisfied-diags.cpp	New test cases for `__is_standard_layout` failures
clang/test/SemaCXX/type-traits-unsatisfied-diags-std.cpp	Added `is_standard_layout` traits and `std::is_standard_layout` tests
clang/lib/Sema/SemaTypeTraits.cpp	Implemented `DiagnoseNonStandardLayoutReason` and helpers
clang/include/clang/Basic/DiagnosticSemaKinds.td	Added `StandardLayout` to unsatisfied-trait notes and reasons

Comments suppressed due to low confidence (2)

clang/lib/Sema/SemaTypeTraits.cpp:2295

[nitpick] The function name 'hasMultipleDataBaseClassesWithFields' is a bit awkward—consider renaming to something like 'hasMultipleBasesWithDataMembers' for clarity and consistency.

static bool hasMultipleDataBaseClassesWithFields(const CXXRecordDecl *D) {

clang/test/SemaCXX/type-traits-unsatisfied-diags.cpp:535

No test covers the 'multiple base classes with data members' path; consider adding a case where a type derives from two bases each with data members to exercise the corresponding diagnostic.

// end of standard_layout_tests

cor3ntin · 2025-06-12T09:35:09Z

Thanks a lot for working on that.

Can you add tests for all of these examples?
https://eel.is/c++draft/class.prop#11

In particular

If there are multiple direct base classes, a type is not a standard layout
If in the inheritance hierarchy, there are multiple classes with members, it is not standard-layout.
We have CXXRecordDecl::hasDirectFields() and CXXRecordDecl::getStandardLayoutBaseWithFields that should help

snarang181 · 2025-06-12T13:13:53Z

Thanks a lot for working on that.

Can you add tests for all of these examples? https://eel.is/c++draft/class.prop#11

In particular

If there are multiple direct base classes, a type is not a standard layout

If in the inheritance hierarchy, there are multiple classes with members, it is not standard-layout.
We have CXXRecordDecl::hasDirectFields() and CXXRecordDecl::getStandardLayoutBaseWithFields that should help

@cor3ntin, I added more inheritance related tests. Kindly review, thanks!

cor3ntin · 2025-06-12T14:06:55Z

clang/test/SemaCXX/type-traits-unsatisfied-diags.cpp

I think this case needs more explanation.
maybe "because both BaseC and (an indirect) base class BaseA have data members"

I have added more descriptive diagnostics. Note that I was not able to use CXXRecordDecl::getStandardLayoutBaseWithFields as the function asserts a standard layout to begin with. As a result, I had to manually walk the inheritance hierarchy to expose this.

snarang181 · 2025-06-13T20:42:04Z

Something broke while resolving merge conflicts. Getting it back to a steady state and will mark it ready for review again. Apologies.

Add diagnostic test cases

indirect base class inheritance

snarang181 · 2025-06-14T06:21:04Z

Closing in favor of #144161

snarang181 changed the title ~~Add std layout diagnostics~~ [Clang] Explain why is_standard_layout evaluated to false Jun 11, 2025

snarang181 marked this pull request as ready for review June 11, 2025 15:18

llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Jun 11, 2025

snarang181 mentioned this pull request Jun 11, 2025

[Clang] Explain satisfaction failure for more type traits #141911

Open

snarang181 requested review from Copilot, cor3ntin and erichkeane June 11, 2025 20:34

Copilot AI reviewed Jun 11, 2025

View reviewed changes

snarang181 force-pushed the explain-std-layout branch from 6b17d6c to 6b33849 Compare June 12, 2025 13:12

snarang181 self-assigned this Jun 12, 2025

cor3ntin reviewed Jun 12, 2025

View reviewed changes

snarang181 force-pushed the explain-std-layout branch 2 times, most recently from 36b26a2 to 0754b80 Compare June 13, 2025 15:50

snarang181 requested review from AaronBallman and cor3ntin June 13, 2025 15:50

snarang181 force-pushed the explain-std-layout branch 3 times, most recently from 044be2b to ea6ad7d Compare June 13, 2025 19:43

snarang181 marked this pull request as draft June 13, 2025 20:41

snarang181 added 2 commits June 13, 2025 22:54

Add std layout diagnostics

6f787e1

Add diagnostic test cases

Add more descriptive messages for

40ac423

indirect base class inheritance

snarang181 force-pushed the explain-std-layout branch from ea6ad7d to 40ac423 Compare June 13, 2025 21:05

snarang181 closed this Jun 14, 2025

snarang181 deleted the explain-std-layout branch June 14, 2025 09:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Clang] Explain why is_standard_layout evaluated to false #143722

[Clang] Explain why is_standard_layout evaluated to false #143722

snarang181 commented Jun 11, 2025 •

edited

Loading

Uh oh!

llvmbot commented Jun 11, 2025

Uh oh!

snarang181 commented Jun 11, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

cor3ntin commented Jun 12, 2025

Uh oh!

snarang181 commented Jun 12, 2025

Uh oh!

cor3ntin Jun 12, 2025

Uh oh!

snarang181 Jun 13, 2025

Uh oh!

snarang181 commented Jun 13, 2025

Uh oh!

snarang181 commented Jun 14, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

[Clang] Explain why is_standard_layout evaluated to false #143722

[Clang] Explain why is_standard_layout evaluated to false #143722

Conversation

snarang181 commented Jun 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Jun 11, 2025

Uh oh!

snarang181 commented Jun 11, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

cor3ntin commented Jun 12, 2025

Uh oh!

snarang181 commented Jun 12, 2025

Uh oh!

cor3ntin Jun 12, 2025

Choose a reason for hiding this comment

Uh oh!

snarang181 Jun 13, 2025

Choose a reason for hiding this comment

Uh oh!

snarang181 commented Jun 13, 2025

Uh oh!

snarang181 commented Jun 14, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

snarang181 commented Jun 11, 2025 •

edited

Loading